Skip to content

fix: respect nullability in nested struct accessors - #967

Merged
wgtmac merged 3 commits into
apache:mainfrom
advancedxy:codex/fix-nested-struct-accessor
Oct 10, 2026
Merged

wgtmac merged 3 commits into
apache:mainfrom
advancedxy:codex/fix-nested-struct-accessor

Conversation

@advancedxy

Copy link
Copy Markdown
Contributor

Summary

  • Carry schema nullability alongside each nested accessor position, including the leaf field.
  • Return null when an optional parent struct is null, and report an error when a required parent or leaf field is null.
  • Preserve the direct access paths for shallow fields while handling deeper paths, non-struct parents, and field errors.
  • Extend accessor tests across one- to four-level paths and align the evaluator test schema with its null input.

Testing

  • pre-commit on all changed files: passed
  • schema_test, expression_test, arrow_test, and eval_expr_test: 4/4 passed

AI assistance

OpenAI Codex assisted with the implementation, refactoring, and test scaffolding. The changes were reviewed against the schema accessor behavior and validated with the tests above.

@advancedxy

Copy link
Copy Markdown
Contributor Author

@wgtmac would you mind to take a look at this? Agent found this problem during our internal integration of iceberg-cpp, which might return null for required field rather than checked or return an error. Java's accessor also has similar problem but it optimize nested access with Position/Position2/Position3 Accessors, which might mitigate this issue.

@wgtmac wgtmac left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Thanks for fixing this, @advancedxy.

@wgtmac
wgtmac merged commit 66d7ef5 into apache:main Oct 10, 2026
15 checks passed
@advancedxy

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing and merging @wgtmac.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants